Skip to content

REF: remove take_1d alias of take_nd #39731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 10, 2021

Conversation

jorisvandenbossche
Copy link
Member

@jreback as discussed in #39692

Since this is just an alias, this is certainly fine in theory. Although now doing it, I think there is still some value of the alias to signal that it is taking a 1D array which can provide useful context when reading code that uses this.

@jreback
Copy link
Contributor

jreback commented Feb 10, 2021

@jreback as discussed in #39692

Since this is just an alias, this is certainly fine in theory. Although now doing it, I think there is still some value of the alias to signal that it is taking a 1D array which can provide useful context when reading code that uses this.

right . i think your next take_1d_array will make more sense (and consider actually naming IT take_1d)

@jreback jreback added this to the 1.3 milestone Feb 10, 2021
@jorisvandenbossche
Copy link
Member Author

Yes, but I am not directly planning on using the take_1d_array in places where I renamed take_1d here to take_nd (that would need checking of each of those cases whether they can guarantee already to be an array)

@jreback
Copy link
Contributor

jreback commented Feb 10, 2021

Yes, but I am not directly planning on using the take_1d_array in places where I renamed take_1d here to take_nd (that would need checking of each of those cases whether they can guarantee already to be an array)

sure but its more clear now. thanks.

@jreback jreback merged commit 0c18cc6 into pandas-dev:master Feb 10, 2021
@lithomas1
Copy link
Member

@jreback @jorisvandenbossche This is causing benchmarks CI to fail?
ImportError: cannot import name 'take_1d' from 'pandas.core.algorithms' (/home/runner/work/pandas/pandas/pandas/core/algorithms.py)

@jreback
Copy link
Contributor

jreback commented Feb 11, 2021

@lithomas1 can u show where
this passed in master

@lithomas1
Copy link
Member

@jreback
Copy link
Contributor

jreback commented Feb 11, 2021

ahh the benchmarks

ok - care to do a PR to fix

afeld added a commit to afeld/pandas that referenced this pull request Feb 11, 2021
@afeld afeld mentioned this pull request Feb 11, 2021
4 tasks
@jorisvandenbossche jorisvandenbossche deleted the ref-take_1d-alias branch February 11, 2021 07:42
@jorisvandenbossche
Copy link
Member Author

Whoops, sorry forgot to search/replace in the benchmarks

jorisvandenbossche pushed a commit that referenced this pull request Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants